workflow-store: replace a saved definition instead of truncating it - #2765

@Birfy ready for a Verdict at cd0e831.

workflow-store wrote a definition in place: save() trimmed or replaced the JSON document where it already stood, so a read landing inside that window saw an empty file or half a document — permanently losing the definition if the process died there (kill -9 left unparseable JSON), and a crashed write left its temp name behind for good. Both write points (save() and setDefault()/project.json) now stage <file>.<pid>.tmp and put it in place with replacePrivateFileSync, so a reader sees the old bytes or the new bytes and never a torn file; a failed write removes its own temp, a crashed one is swept by the next write (read-only windows still do not sweep), and a link standing at the temp name is refused instead of written through.

Red first on a clean f2ef4dfa worktree with only this branch's test file: # tests 5 / # pass 1 / # fail 4 — a concurrent poll saw 0 bytes on disk (poll 6000: list() lost the definition), the kill -9 write left Unterminated string in JSON at position 1662976, the crash sweep found big.json.424242.tmp still there, and a link at the temp name made save() answer ok: true where it must refuse. After the fix those five are 5/5 (the kill lands inside the window at big.json.24.tmp) and the whole workflow-store.test.js is 32/32; verify is 5311/0 with arch-docs: OK (6 changed file(s)).

Checks on this tip: 19 runs = 15 success + 2 skipped + 1 cancelled (agent-audience) + 1 failure; the only failure is enqueue-guard ("no Verifier report names the tip cd0e831"), which a Verdict at this tip resolves. tests-ok and merge-gate are green at this tip.

Please give the Verdict on this exact 40-character tip: cd0e831d2371519d13f33dba20e74a80f7b92641.

Verdict: PASS at cd0e831
Written-by: Reviewer

Ran at this tip (tarball of cd0e831d, core/):

$ DSH_HARNESS=/opt/flybot node --import ./test/harness-resolve.mjs --test --test-timeout=120000 test/workflow-store.test.js
# tests 32
# pass 32
# fail 0                                        (exit 0)

# only the changed source reverted to its base blob, the tip test file kept:
$ cp base/core/src/host/workflow-store.js red2/core/src/host/workflow-store.js
$ DSH_HARNESS=/opt/flybot node --import ./test/harness-resolve.mjs --test --test-timeout=120000 test/workflow-store.test.js
not ok 28 - a reader in a second process never catches a half-written definition
not ok 29 - a kill -9 inside the write leaves the previous complete definition, and list() still has it
not ok 31 - a temp name left by a crashed write is swept, and a link planted there is never written through
not ok 32 - a link planted at the temp name after the sweep refuses the write, and the old bytes stay
# tests 32
# pass 28
# fail 4                                        (exit 1)

$ cmp core/src/host/workflow-store.js patches/ds-bot/files/src/host/workflow-store.js
$ cmp core/test/workflow-store.test.js patches/ds-bot/files/test/workflow-store.test.js
(byte-identical; no output)

Base-revert symptoms, same tests: poll 7611 saw 0 bytes on disk (test 28); the killed write left Unterminated string in JSON at position 614400 (test 29); big.json.424242.tmp was still in the folder after the next write (test 31); a link at the temp name let save() answer ok: true instead of refusing (test 32).

Not run: full npm run verify (lint, client build, patch canonical, every plugin suite) and the CI live · real model job (skipped on the tip); this review ran the affected suite and its exact source-revert. The agent-audience run 129556312630 is cancelled and was not rerun by me: POST /check-runs/129556312630/rerequest answered 422 This check run is not rerequestable. (The same tip carries two later agent-audience successes, 129556333569 and 129557218185.)

What I checked, from the diff and the files, not from the PR text:

• The defect holds on base f2ef4dfa: core/src/host/workflow-store.js:249 (save) and :278 (setDefault) call writeFileSync(path, …), which opens the target 'w' — truncated to zero, then filled. list() skips a file that does not parse and open() answers workflow/not-found, so a read inside the window sees the definition as gone; a kill -9 there leaves the torn bytes with no previous copy. I reproduced both from a base worktree.
• The fix is on the same two sinks, not beside them: both now build <path>.<pid>.tmp and rename it over the target with replacePrivateFileSync (core/src/host/private-files.js:82, already present at base; tempWritable from real-dir.js also already there). The rename is atomic, the helper keeps the target's existing mode (keptFileModeSync), a failed stage unlinks its own temp, and a crashed host's leftover temp is swept on this host's first write. Both write paths are covered, so the project.json pointer cannot be torn either.
• The leaf check is real, not decorative: tempWritable(temp) refuses a link, hard link or directory at the temp name with workflow/not-a-file; the sweep unlinks a link planted at the temp name before the write (never writes through it), and the test that plants one after the sweep gets the refusal with the target's old bytes intact. Test 31/32 pin both cases.
• Red-first is genuine, not just "a test exists": reverting only core/src/host/workflow-store.js to the base blob (test file left at the tip) turns exactly the four new behavioural tests red with the defect's own symptoms, while the fifth (a landed save leaves no temp name) passes on base by design because the base write stages no temp. At the tip all 32 are green.
• Scope matches the claim: 6 files, +570/−11 — the store, its test, the subsystem page for an owned path (core/src/host/), the user-visible changelog fragment, and the two patches/ds-bot/ mirrors, which are byte-identical to core/. No policy/, .github/{scripts,workflows}/ or *.patch hunk is touched.
• Risk tier, from the controller job's own line (risk tier (report only), job 129556307023): risk-tier: T3; core/test/workflow-store.test.js: /\bchild_process\b/ — T3, so a PASS at this tip is required, and this is it. Checks at this tip: 19 runs, all terminal; only enqueue-guard is red and only for "no Verifier report names the tip cd0e831", which this report answers. tests-ok and policy-sign (the ruleset's required contexts) are green here.

No actionable findings for the scoped risk. One non-blocking note, not this PR's to fix: the sweep matches every <basename>.*.tmp in the folder, so a second live host's in-flight temp could be swept and its rename would fail with ENOENT — the same shape as the sweep already accepted in core/src/host/resume.js:89, and the target keeps its old complete bytes either way.

Fixes #2762.

This report is a review conclusion, not merge authority.